Update dependencies, require node 20 #775
Conversation
tadzik
force-pushed
the
tadzik/dep-updates
branch
from
ec8ee33 to
b68e76c
Compare
tadzik
force-pushed
the
tadzik/dep-updates
branch
from
b68e76c to
f280497
Compare
|
Looks sane to me, but the postgres integration test fail seems legit? edit: oh wait, looks like it's just busted, so probably not related to this PR. |
|
It's worth considering updating our matrix-appservice-bridge here and dropping NeDB while at it – we'd need to support a DB migration though. |
| @@ -38,7 +38,7 @@ jobs: | |||
| runs-on: ubuntu-latest | |||
There was a problem hiding this comment.
Ideally we should make sure this works before merging.
| @@ -25,14 +25,6 @@ const log = new Logger("substitutions"); | |||
| const ATTACHMENT_TYPES = ["m.audio", "m.video", "m.file", "m.image"]; | |||
| const PILL_REGEX = /<a href="https:\/\/matrix\.to\/#\/(#|@|\+)([^"]+)">([^<]+)<\/a>/g; | |||
|
|
|||
| /** | |||
There was a problem hiding this comment.
Assuming this is now part of the functionality.
There was a problem hiding this comment.
Correct. I've now added tests to verify this.
It needed the default timeout increased, 2 seconds was not enough to apply DB migrations. Should be good now. |
changelog.d/775.misc
Outdated
| @@ -0,0 +1 @@ | |||
| Require node 20, update dependencies. | |||
There was a problem hiding this comment.
This needs to be a .removal, as we're removing support. I'd put in stronger wording to say that Node 16 and 18 are no longer supported, and users need to update to Node 20.
I wouldn't both stating that dependencies are updated, it doesn't help the user any to know that.
There was a problem hiding this comment.
Good points. Changed in 7f3cdec
| @@ -13,6 +13,6 @@ jobs: | |||
| - name: Use Node.js | |||
There was a problem hiding this comment.
N.B. Remember to update the Dockerfile.
Signed off by: Tadeusz „tadzik” Sośnierz <signoff at tadzik dot net>